-
Notifications
You must be signed in to change notification settings - Fork 14
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Send files via pathanmes #19
base: clos-everywhere
Are you sure you want to change the base?
Send files via pathanmes #19
Conversation
src/network.lisp
Outdated
when value | ||
if pathname-p | ||
collect (cons (kebab:to-snake-case key) value) | ||
else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think here you are breaking the previous logic or this else
belong to if
but having a wrong indentation. Seems there is a problem with loop indentation in emacs, because for me in this small example it also indents else
on the same level as when
:
CL-USER> (loop for i in (list NIL NIL 1 2 3 4 5)
when i
if (< i 3)
collect (list :before-else i)
else
collect (list :else i))
((:BEFORE-ELSE 1) (:BEFORE-ELSE 2) (:ELSE 3) (:ELSE 4) (:ELSE 5))
Let's rewrite this code to make it more readable?
Also, I don't like an explicit pathname-p argument. We already have a pathname in the options. So we can decide if we wanna to make a multipart request right in the make-request
itself. I'd add a helper function (multipart-request-required-p options)
which will return true if at least one of options is pathname
. Also pathname-p
argument to process-options
should be renamed to return-alist-p
- this way it will be more obvious that this function can return plist or alist depending on this mode.
@rtmpdavid will you update this PR or I can do this myself? |
@svetlyak40wt I'll have some free time to do it this weekend. |
@svetlyak40wt I've updated the pr. |
416c89e
to
9ed8cd7
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cool, I'd change find-if
to some
. But this you can decide youself.
collect (kebab:to-snake-case key) | ||
and | ||
collect value)) | ||
(multipart-required (find-if #'pathnamep options)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Probably it will be better to use `(some #'pathnamep options) here, because we don't need a pathname as the result:
CL-USER> (some #'pathnamep (list 1 2 #P"foo" #P"bar" 5 6))
T
CL-USER> (find-if #'pathnamep (list 1 2 #P"foo" #P"bar" 5 6))
#P"foo"
@rtmpdavid I've just merged a huge PR where a new ASDF system cl-telegram-bot2 has been added. It supports sending media files using pathnames. Probably we should close this PR, because I have no plans to improve old API. |
Dexador can automatically create multi-part post requests when given pathnames in content.